elements@1 - Auto-load custom-elements (web-components) from extensions - #34348
elements@1 - Auto-load custom-elements (web-components) from extensions#34348totten wants to merge 1 commit into
Conversation
|
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
|
@totten this looks really cool. Couple questions:
|
Correct.
Compare to autoloading with PHP-Composer:
Minification? There may be a miscommunication. The file-extension question is a squishy thing, though. I'm not saying EDNI(?) ("Everyone Definitely Needs It!"); it does merit more conversation. It's really a separate thread, though. (The support in here is "compatible with" or "prepared for" an |
| $mimeType = 'text/javascript'; | ||
| $registry = $this->getAll(); | ||
| $content = strtr($this->getTemplate(), [ | ||
| 'ELEMENTS_REGISTRY' => json_encode($registry, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES), |
There was a problem hiding this comment.
Normally we also don't escape unicode.
civicrm-core/CRM/Utils/JSON.php
Line 45 in 6eef79a
|
@totten cool, so how will translatable strings get extracted and sent to the client? civicrm-core/CRM/Core/Resources/CollectionTrait.php Lines 115 to 119 in 92a3299 And with Angular it happens here: civicrm-core/Civi/Angular/Manager.php Lines 295 to 313 in 82b0e85 |
That's a really good question, and I'm not certain the answer. The code-style of ESM would tend to emphasize using static imports. If you were doing pure ESM without any other tooling, you might have an adjacent data-files: import ts from "myextension/element/hello-world.strings.js";The strings need to vary by user locale, but you could address that by mapping the path to asset-builder. I'm nervous about increasing the #requests multiplicatively. (One JS file-request per custom-element? For (say) 30 elements, that's 30 requests... fine? But for JS+CSS+strings, then it's 90 requests. Hmm...) Maybe a better balance is to fetch the strings per-extension or per-folder? (So import ts from "myextension/element/ALL-STRINGS.js"; ## Handled via asset-builderBut there are still a more angles to work out. (Caching/locale) And I kinda suspect it'll be easier to work out if we get the strings with a dynamic import, e.g. const ts = await import(CRM.url('civicrm/ajax/strings?target=' + import.meta.resolve('.') + "&locale="+...));
const ts = await import(CRM.stringsUrl('myextension'));
const ts = await import(CRM.stringsUrl(import.meta));
const ts = await import(CRM.stringsUrl(import.meta)) with { type: "json" };
const ts = await CRM.importStrings(import.meta);So getting toward the end there, maybe strings-import formulation which:
|
|
This seems great. Small quibble: the mixin is called |
|
In the MDN docs for Web Components, they point to using The reference docs for I guess... if we're going to autoload the
(Actually... if you just had |
|
|
||
| const ASSET = 'elements.js'; | ||
|
|
||
| const SUBDIR = 'element'; |
There was a problem hiding this comment.
| const SUBDIR = 'element'; | |
| const SUBDIR = 'elements'; |
There was a problem hiding this comment.
Yeah, it is slightly annoying how inflections vary within Civi. My first reaction was: "Right, consistency is good; fine, let's rename it". And I kind of like how ./elements/ resembles ./templates/. To implement this, it does need a slightly bigger commit:
- Prefer plural (
elements@1+./elements/): 8bf36ec
But I'm a sucker for this kind of thing. If we're going for consistency, then one should look the existing top-level folders and mixins:
- Folder Names: You can argue that top-levels look more plural or more singular. Really, less than half of folders (~9) indicate a free and unambiguous choice of singular vs plural. (To my eye, those lean plural.) The majority are blurry cases (~16) with abbreviations and proper-names. (To my eye, those lean singular.)
- Mixin Names: For mixins... the "unambiguous choices" and the "blurry cases" both lean toward "singular" (though plural cases do appear as well).
So... I think that singular is more consistent, in which case -- here's the commit going the other direction:
- Prefer singular (
element@1+./element/): 1decafd
Of course, there's a lot that we inherit from upstream ecosystems -- bower_component[s]/, vendor[s]/, js/, css/, bin[s]/, etc -- and they don't point in consistent directions either...
There was a problem hiding this comment.
Right, I fear we will have this argument until the end of time...
My view is consistent at least: plurals are more human readable, and thereby well worth one extra character. Anything like this we add should use plurals.
There was a problem hiding this comment.
My vote would also be for elements which is consistent with the sibling settings and templates directories.
But whichever we do we should merge this PR and get on with our lives.
|
Couple of ideas for translation: clientside: use a web component for lazy fetching
serverside: have a dedicated smarty template for each component - render it serverside (including translations) and then send to the client (as an async request? by including in big JS var of rendered templates?
I'm wondering whether we might want to have both available 🤔 |
|
Apologies if this is an obvious question, but how would a theme overwrite At the moment there is a directory of 'component' css files in River (that obvs aren't web components, just thematic groups), it's been a hope that as Civi moves to Web Components, the css files for those can be managed in one place in the theme layer (rather than lots of quite specific directory names pointing to lots of css files dotted across Civi. |
I think the idea would be this wouldn't be possible or required. Any css included in the web component itself should be extremely minimal, self-contained, and only functional not thematic. Theming would then apply on top. |
|
Here's a stab at the serverside translation approach: https://github.com/ufundo/civicrm-core/pull/12/changes There's some crossover with the loader here - I think it shouldn't be too hard to reconcile, though I am stumbling on how "core" web components would work with this mixin loader. And possibly some architecture should go into core that can't be mixin-ed (e.g. new route?). |
How would you separate 'only functional' from 'thematic' css? This is the 189-line SCSS that Bootstrap 5 has to define radio/checkboxes: https://github.com/twbs/bootstrap/blob/main/scss/forms/_form-check.scss - that's arguably mostly 'functional' - styling is abstracted via variables. Civi's radio-button component css might be longer given requirements for specific count of columns, help text, error messaging, etc. E.g. the gap between a radio button and its label is functional but themes will vary the size of that. There's multiple ways to do that - with margin left/right, padding, or gap if the entire thing is wrapped in flexbox or grid. If a theme has an opinion on that it might have to replace lots of 'functional' radio/checkbox css - and then the files would load twice? I'm not saying component css files need to be over-writeable in the same way extension's css files currently are - but if they're not, then they'd either want to use css variables to support broad customisation, or themes are will have to duplicate a lot of that css. |
|
Random thoughts:
|
Nice! |
|
@totten so now we need to require https://github.com/civicrm/html-autoloader in |
I think so. Though first I think we'd need to publish a composer package? Can't we just include https://github.com/civicrm/html-autoloader/blob/main/src/HtmlAutoloader.js here @totten ? |
Overview
Define a file-naming convention for "Custom Elements" (aka "Web Component"s) based on
*.js,*.mjs, and/or*.cssfiles. Implement lazy-loading with ECMAScript module (ESM) support.(Ping @ufundo @colemanw. This is a re-spin of #34325 which adds CSS support, changes the name from
component-jsto justelements, and adds some test-coverage.)Technical Details: Consumers
As a consumer of a custom-element, you simply use the tag... somewhere. It can be Quickform or AngularJS or jQuery or whatever. For example:
But how is this new tag defined?
Technical Details: Providers
As the provider of a custom element, you will do the following:
info.xmland enable<mixin>elements@1</mixin>.element/TAG-NAME.js.For example, here is an implementation of
<hello-world>:Since this supports ECMAScript modules, you can import helpers from other files, as in:
This is implemented as a mixin, and it should be amenable for backporting on 5.63+.
Comments
The general approach is this:
AssetBuilderto create a semi-static file,elements.js, which stores an index of all these CustomElements.MutationObserverto determine when new elements are needed.MutationObservershared by all these CustomElements.Limitations worth considering:
It uses lazy-loading (i.e. on-demand). This means you may perceive some flashing/delay as components render.
(I imagine some convention could help manage that... but I'm not sure what such a convention should be...)
This doesn't specifically implement bundling, and we can defer that question. However, experiments based on this technique are promising. The key thing is that the registry maps
['my-element' => [...jsFile, cssFile...]]. This is many-to-one. Multiple elements can map to the same file, and the file is only be loaded once. So it should be agreeable to various tricks/techniques (with trade-offs on techniques).Translations - We probably need some more logic to pull in strings for these
import()d JS files.